[JEWEL-1075] Fix crash in ScrollableContainer when used with IntrinsicSize#3471
Open
DanielSouzaBertoldi wants to merge 1 commit intoJetBrains:masterfrom
Open
[JEWEL-1075] Fix crash in ScrollableContainer when used with IntrinsicSize#3471DanielSouzaBertoldi wants to merge 1 commit intoJetBrains:masterfrom
DanielSouzaBertoldi wants to merge 1 commit intoJetBrains:masterfrom
Conversation
…cSize When a VerticallyScrollableContainer or HorizontallyScrollableContainer was placed inside a layout querying intrinsic measurements (e.g., Column(Modifier.width(IntrinsicSize.Max))), Compose's default MeasurePolicy fallback re-ran the measure block with Constraints.Infinity. The scrollbar measurement then called Constraints.fixedHeight(Int.MAX_VALUE), which is an illegal Constraints value and threw an IllegalArgumentException. Fix the crash by converting the Layout SAM lambdas in ScrollableContainer and the scrollbar measure policies in Scrollbar to explicit MeasurePolicy objects with proper intrinsic overrides. The scrollbar policies report their thumb thickness as intrinsic width/height on the fixed axis and 0 on the scroll axis. ScrollableContainer queries scrollbar intrinsic sizes first, then measures content and scrollbars with concrete constraints, avoiding any unbounded intermediate measurement. Additionally add LazyColumn-style check() assertions that throw IllegalStateException when the scroll axis has an unbounded constraint during actual layout, giving developers a clear error message instead of a cryptic crash deep in Constraints internals. Tests covering all edge cases are added to ScrollableContainerTest: intrinsic-size parents no longer crash, containers report correct intrinsic widths (including scrollbar space on macOS AlwaysVisible), scrolling still works after the fix, and unbounded-axis nesting produces the expected IllegalStateException with a descriptive message.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
(this description was generated by
jewel-pr-preparer)Context
VerticallyScrollableContainer(and its horizontal counterpart) crashed withIllegalArgumentExceptionwhen being used inside a Composable that expectsIntrincisSize. This is the crash stacktrace:Compose's default
MeasurePolicyintrinsic fallback re-runs the measure block withConstraints.Infinityon the queried axis. The old implementation used a SAM lambda forLayout { }, so the measure block would run during intrinsic queries. Inside that block, the scrollbar was being measured withConstraints.fixedHeight(Int.MAX_VALUE), an illegal Constraints value that throws immediately.Changes
ScrollableContainer: instead of measuring scrollbars withConstraints.fixedHeight/Width(Int.MAX_VALUE)to query their size, the implementation now queries scrollbar intrinsic sizes first by converting the Layout SAM lambda to an explicitMeasurePolicyobject with proper intrinsic overrides, then measures content, then measures scrollbars with concrete dimensions.Layoutlambdas inScrollbar.ktwere themselves using SAM lambdas with no intrinsic overrides as well, so Compose's default fallback would re-run their measure blocks withConstraints.Infinity, causing the same crash one level deeper. BothverticalMeasurePolicyandhorizontalMeasurePolicywere converted to explicitMeasurePolicyobjects that reportthumbThicknessas the intrinsic size on their fixed axis and 0 on the scroll axis.check()assertions (matchingLazyColumn's behavior) that throwIllegalStateExceptionwith a descriptive message when the scroll axis has an unbounded constraint during actual layout, giving developers clear guidance instead of a cryptic crash deep in Compose internals.ScrollableContainerTestcovering: no crash inIntrinsicSize.Max/IntrinsicSize.Minparents, correct reported intrinsic dimensions (including scrollbar space for the macOSAlwaysVisiblestyle), scrolling still works after the fix, and unbounded-axis nesting produces the expected IllegalStateException.Release notes
Bug fixes
VerticallyScrollableContainerandHorizontallyScrollableContainerno longer crash withIllegalArgumentExceptionwhen placed inside a layout that queries intrinsic measurements.